Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: Use invalid prop for most inputs #1240

Merged
merged 13 commits into from
May 29, 2024

Conversation

alimpens
Copy link
Contributor

Questions:

  • Does omitting attributes from types constitute a breaking change?
  • Should Search Field and Switch get an invalid prop? Are there situations in which these can be invalid?

@alimpens alimpens requested review from dlnr, VincentSmedinga and RubenSibon and removed request for dlnr May 24, 2024 15:16
@github-actions github-actions bot temporarily deployed to demo-DES-733-use-invalid-prop May 24, 2024 15:46 Destroyed
# Conflicts:
#	packages/react/src/TextArea/TextArea.test.tsx
#	packages/react/src/TextArea/TextArea.tsx
#	packages/react/src/TextInput/TextInput.test.tsx
#	packages/react/src/TextInput/TextInput.tsx
@github-actions github-actions bot temporarily deployed to demo-DES-733-use-invalid-prop May 27, 2024 09:34 Destroyed
@VincentSmedinga
Copy link
Contributor

  • Does omitting attributes from types constitute a breaking change?

Copied this to a change, preventing quoting high-level comments.

  • Should Search Field and Switch get an invalid prop? Are there situations in which these can be invalid?

Well, there are theoretical situations. Both implement the HTMLInputElement interface which has properties like required and minLength that may fail a validation rule.

We offer Switch to toggle UI states only, so it can never become invalid in practice. We can remove all validation fields from the type, or not even use the input interface at all.

A Search Field might have a minimum length of e.g. 3 characters to facilitate meaningful results, although I would not expect it to turn red if the field gets blurred with a content of less than 3 characters. On the other hand, if people press Enter or click the Search Button, there should be a way to inform them of this rule.

So maybe yes for Search Field, and tighten the props for Switch?

@github-actions github-actions bot temporarily deployed to demo-DES-733-use-invalid-prop May 29, 2024 07:46 Destroyed
@alimpens alimpens changed the title feat: Use invalid prop for most inputs feat!: Use invalid prop for most inputs May 29, 2024
@github-actions github-actions bot temporarily deployed to demo-DES-733-use-invalid-prop May 29, 2024 07:51 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-733-use-invalid-prop May 29, 2024 08:11 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-733-use-invalid-prop May 29, 2024 11:32 Destroyed
@VincentSmedinga VincentSmedinga merged commit 9477186 into develop May 29, 2024
4 checks passed
@VincentSmedinga VincentSmedinga deleted the feature/DES-733-use-invalid-prop branch May 29, 2024 11:58
@github-actions github-actions bot mentioned this pull request Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants